Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Sandboxing #417

Closed
wants to merge 15 commits into from
Closed

Sandboxing #417

wants to merge 15 commits into from

Conversation

edg-l
Copy link
Member

@edg-l edg-l commented Jan 11, 2024 •
edited
Loading

Design:

  • A new IsolatedExecutor which spawns the cairo-executor binary and manages it, sending and receiving messages via ipc
  • A new binary called cairo-executor which is spawned by IsolatedExecutor and receives and sends messages via ipc, executing programs and returning the results. It also forwards the syscalls via ipc, so ultimately the syscallhandler is implemented by the host side. The cairo-executor implements it as a bridge for message passing.

There is an example called "sandbox" showing how it works:

    let path = Path::new("programs/examples/hello_starknet.cairo");

    let contract = compile_path(
        path,
        None,
        CompilerConfig {
            replace_ids: true,
            ..Default::default()
        },
    )
    .unwrap();

    let path = std::env::current_dir()
        .unwrap()
        .join("target/debug/cairo-executor");
    let sandbox = IsolatedExecutor::new(&path).unwrap();

    let entry_point = contract.entry_points_by_type.external.get(0).unwrap();

    let mut handler = SyscallHandler;

    // run program blocks, so ideally each isolated executor is ran on its own thread.
    let result = sandbox
        .run_program(
            contract.extract_sierra_program().unwrap(),
            vec![1.into()],
            Some(u128::MAX),
            entry_point.function_idx,
            &mut handler,
        )
        .unwrap();
    println!("{:#?}", result);

    // run more programs etc

Description

Description of the pull request changes and motivation.

Checklist

  • Linked to Github Issue
  • Unit tests added
  • Integration tests added.
  • This change requires new documentation.
    • Documentation has been added/updated.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 11, 2024 •
edited
Loading

Benchmarking results

Benchmark for program factorial_2M

Open benchmarks
Command Mean [s] Min [s] Max [s] Relative
Cairo-vm (Rust, Cairo 1) 17.714 ± 0.131 17.546 18.022 36.51 ± 0.28
cairo-native (embedded AOT) 2.263 ± 0.008 2.253 2.276 4.66 ± 0.02
cairo-native (embedded JIT using LLVM's ORC Engine) 1.386 ± 0.006 1.378 1.398 2.86 ± 0.01
cairo-native (standalone AOT) 0.700 ± 0.002 0.698 0.703 1.44 ± 0.01
cairo-native (standalone AOT with -march=native) 0.485 ± 0.001 0.484 0.488 1.00

Benchmark for program fib_2M

Open benchmarks
Command Mean [s] Min [s] Max [s] Relative
Cairo-vm (Rust, Cairo 1) 17.181 ± 0.078 17.023 17.319 588.67 ± 3.22
cairo-native (embedded AOT) 0.983 ± 0.009 0.971 1.006 33.68 ± 0.34
cairo-native (embedded JIT using LLVM's ORC Engine) 0.926 ± 0.005 0.920 0.936 31.71 ± 0.19
cairo-native (standalone AOT) 0.041 ± 0.000 0.041 0.042 1.41 ± 0.01
cairo-native (standalone AOT with -march=native) 0.029 ± 0.000 0.029 0.029 1.00

Benchmark for program logistic_map

Open benchmarks
Command Mean [s] Min [s] Max [s] Relative
Cairo-vm (Rust, Cairo 1) 1.623 ± 0.011 1.608 1.642 23.29 ± 0.17
cairo-native (embedded AOT) 1.139 ± 0.005 1.131 1.149 16.34 ± 0.08
cairo-native (embedded JIT using LLVM's ORC Engine) 0.988 ± 0.013 0.975 1.024 14.18 ± 0.19
cairo-native (standalone AOT) 0.109 ± 0.000 0.109 0.109 1.57 ± 0.00
cairo-native (standalone AOT with -march=native) 0.070 ± 0.000 0.069 0.070 1.00

@codecov-commenter
Copy link

codecov-commenter commented Jan 11, 2024 •
edited
Loading

Codecov Report

Attention: 385 lines in your changes are missing coverage. Please review.

Comparison is base (5e00369) 76.67% compared to head (f4ee87c) 75.65%.

❗ Current head f4ee87c differs from pull request most recent head 49e5162. Consider uploading reports for the commit 49e5162 to get more accurate results

Files Patch % Lines
src/bin/cairo-executor.rs 0.00% 281 Missing ⚠️
src/sandbox.rs 0.00% 98 Missing ⚠️
src/starknet.rs 0.00% 5 Missing ⚠️
src/execution_result.rs 50.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #417      +/-   ##
==========================================
- Coverage   76.67%   75.65%   -1.03%     
==========================================
  Files         108      110       +2     
  Lines       28241    28622     +381     
==========================================
  Hits        21655    21655              
- Misses       6586     6967     +381

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@edg-l edg-l marked this pull request as ready for review January 22, 2024 14:49
@edg-l edg-l marked this pull request as draft January 23, 2024 14:53
@igaray
Copy link
Collaborator

igaray commented Jan 31, 2024

Moved over to Starknet in Rust in lambdaclass/starknet_in_rust#1209

@igaray igaray closed this Jan 31, 2024
@igaray igaray mentioned this pull request Jan 31, 2024
Draft
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@igaray igaray Awaiting requested review from igaray igaray is a code owner

@azteca1998 azteca1998 Awaiting requested review from azteca1998 azteca1998 is a code owner

@jrchatruc jrchatruc Awaiting requested review from jrchatruc jrchatruc is a code owner

@entropidelic entropidelic Awaiting requested review from entropidelic entropidelic is a code owner

@fmoletta fmoletta Awaiting requested review from fmoletta fmoletta is a code owner

@Oppen Oppen Awaiting requested review from Oppen Oppen is a code owner

@pefontana pefontana Awaiting requested review from pefontana pefontana is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@edg-l @codecov-commenter @igaray